Fix issues in tableRowFromMessage #36425

reuvenlax · 2025-10-07T18:34:02Z

This PR fixes multiple fundamental issues in tableRowFromMessage. This function is usually used in the dead-letter output from BigQueryIO, though it has other uses as well.

If the BigQuery table has a column named "f" the existing code throws an exception. This is because the TableRow class has an implicit field named "f" which conflicts. The fix is to detect this case and format the TableRow using setF instead. We do this only if an "f" column is detected to maintain backwards compatibility with existing usage.
When converting a TableRow to a protocol buffer, specific field types (e.g. DATE, DATETIME) are encoded using an efficient encoding structure. The existing tableRowFromMessage function didn't know about these encodings, so when converting back to TableRow it simply copied the raw encoded data back. Users of TableRow don't know how to deal with these encodings, so this effectively corrupted the result. In order to properly decode these types, we had to extend the internal format function to take extra information about the BigQuery schema.

In addition to fixing these bugs, many tests were added.

gemini-code-assist · 2025-10-07T18:34:14Z

Summary of Changes

Hello @reuvenlax, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the tableRowFromMessage method to use TableRow.setF() for populating TableRow objects, improving correctness and efficiency. It also includes a minor cleanup by removing an unused import.

Highlights

Intent: The primary intent of this pull request is to refactor the tableRowFromMessage method in TableRowToStorageApiProto.java to use the TableRow.setF() method for populating TableRow objects. This change addresses a previous TODO comment in the code, indicating that setF() is the more correct and efficient way to construct TableRow instances.
Changes Summary: The core change is in sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProto.java. The tableRowFromMessage method has been significantly refactored. Instead of iterating through message fields and calling tableRow.put() for each, it now constructs a List<TableCell> by iterating through the message's field descriptors. Each TableCell is populated with its value using setV() if the field exists and passes CDC column exclusion and field inclusion checks. Finally, the TableRow is created and populated using tableRow.setF(tableCells). Additionally, an unused import for java.util.Objects was removed from sdks/java/io/debezium/src/main/java/org/apache/beam/io/debezium/DebeziumReadSchemaTransformProvider.java.
Reviewer Activity Summary: No reviewer activity has been provided in the context.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

github-actions · 2025-10-07T20:36:25Z

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

Abacn

Thank you!

reuvenlax · 2025-10-08T20:04:20Z

@Abacn Upon adding a unit test, I realized that there are some more fundamental problems with this code (pre existing) - it corrupts certain types. Trying to fix this now.

github-actions · 2025-10-08T20:36:31Z

Assigning reviewers:

R: @m-trieu for label java.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

liferoad · 2025-10-10T15:22:36Z

Fixes #33531

liferoad · 2025-10-11T13:35:28Z

/gemini review

gemini-code-assist

Code Review

This pull request is a significant refactoring to improve how BigQueryIO handles Protobuf messages, specifically to support field names like "f" which conflict with TableRow's internal API. The changes are extensive, introducing SerializableBiFunction to pass schema information, and refactoring TableRowToStorageApiProto to use a more robust setF-based approach for TableRow creation. The introduction of SerializableBiFunctions and new tests for various proto encodings are great additions. My review includes a few minor suggestions for code style and clarity in the new helper classes, and a question about a change in test coverage. Overall, this is a high-quality contribution that improves the robustness of BigQueryIO.

reuvenlax · 2025-10-11T19:20:12Z

/gemini review

reuvenlax · 2025-10-11T19:20:52Z

@Abacn @liferoad here is the updated PR. The issues were deeper than expected, hence the more involved PR (and all the new test coverage)

gemini-code-assist

Code Review

This pull request introduces significant fixes to tableRowFromMessage to handle edge cases and improve data conversion correctness. Specifically, it addresses issues with BigQuery columns named "f" by introducing a fallback to using TableRow.setF, and it correctly decodes special BigQuery types (like DATE, DATETIME) from protocol buffers by leveraging schema information. The changes are extensive, refactoring many parts of the BigQueryIO write path to plumb through the necessary schema information, and introducing a SerializableBiFunction to formatting functions. The addition of SerializableBiFunctions provides useful utilities. The code changes look solid and are well-tested with new unit and integration tests. I have a couple of suggestions for improving exception messages and cleaning up test code.

...le-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIOWriteTest.java

gemini-code-assist · 2025-10-11T19:22:37Z

...ud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProto.java

+              throw new RuntimeException(
+                  "Not implemented yet "
+                      + fieldDescriptor.getMessageType().getName()
+                      + " PARTIAL NAME "
+                      + fieldDescriptor.getMessageType().getName()
+                      + " FIELD NAME "
+                      + prefix
+                      + " CLASS TYPE "
+                      + fieldValue.getClass());
+            }


The exception message here seems to contain debugging information ("PARTIAL NAME", "FIELD NAME", "CLASS TYPE"). It would be better to provide a cleaner, more user-friendly message. Also, consider using UnsupportedOperationException instead of a generic RuntimeException for cases that are not implemented. This applies to similar throw new RuntimeException("Not implemented...") statements in this method for other types like BOOL, INT64, BYTES, and TIMESTAMP.

throw new UnsupportedOperationException( "Converting BigQuery 'DOUBLE' from a protobuf message of type '" + fieldDescriptor.getMessageType().getName() + "' is not supported. Field: " + prefix);

...-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProtoIT.java

Abacn · 2025-10-14T16:48:06Z

Thanks, understand there is a deeper issue in tableRowFromMessage and requires substantial changes, given that there is still 20+ tests breaking (https://github.com/apache/beam/pull/36425/checks?check_run_id=52537069798), I imagine it may take time to clean up all stuff. Alternatively would it be acceptable to adopt the last reviewed version (214e73d) for 2.69.0 ?

cc: @liferoad WDYT?

liferoad · 2025-10-14T16:59:24Z

https://beam.apache.org/releases/javadoc/current/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.Write.html#withFormatRecordOnFailureFunction(org.apache.beam.sdk.transforms.SerializableFunction) is still a workaround for this. So I think fixing with Beam 2.69.0 is not urgent.

reuvenlax · 2025-10-15T19:32:00Z

Agree. We will get this in after the cut.

reuvenlax · 2025-10-22T20:56:27Z

@Abacn all tests now passing

reuvenlax · 2025-10-23T04:42:38Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces significant fixes and refactorings to tableRowFromMessage to address fundamental issues with data type conversions and field name conflicts. The changes correctly handle BigQuery columns named "f" by using setF and ensure proper decoding of special types like DATE and DATETIME by leveraging schema information during conversion from protocol buffers. The introduction of SerializableBiFunction and its helpers is a good way to evolve the API while maintaining backward compatibility. The code is also made more maintainable by centralizing conversion logic. I've found a couple of potential issues related to locale-sensitivity in number formatting and incorrect timestamp formatting that could lead to bugs. Overall, this is a solid improvement.

...ud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProto.java

Abacn

Thanks, understanding the need of plumbing schemainformation led to refactorings. Had one comments hopefully find useful. Have done review with schemainformation/getF fixes, still going through date time changes

...a/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.java

Abacn

Thanks!

(just run /gemini review again to check typos - to my experience the tool is good at finding this kind of issue)

Abacn · 2025-10-30T15:24:30Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces significant and well-executed fixes to tableRowFromMessage, addressing fundamental issues with type decoding and handling of special field names. The introduction of SchemaInformation to guide the conversion from protocol buffers to TableRow is a crucial improvement for correctly handling BigQuery-specific types like DATE and DATETIME. The clever fallback mechanism to handle table columns named "f" by using setFlipstick is a great way to ensure backward compatibility while fixing the underlying issue. The refactoring of type conversion logic into a map of converters in TableRowToStorageApiProto greatly improves the code's structure and maintainability. The extensive addition of tests covering these fixes is commendable. I have one suggestion regarding a DateTimeFormatter implementation that could be improved.

...o/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryUtils.java

github-actions bot added java io gcp labels Oct 7, 2025

liferoad mentioned this pull request Oct 7, 2025

[Test] fix(bigquery): handle field named "f" in tableRowFromMessage #36397

Closed

3 tasks

reuvenlax force-pushed the fix_reverse_f_issue branch from e5e58c1 to 214e73d Compare October 8, 2025 05:06

liferoad requested a review from Abacn October 8, 2025 16:41

Abacn approved these changes Oct 8, 2025

View reviewed changes

github-actions bot added the Next Action: Reviewers label Oct 8, 2025

reuvenlax force-pushed the fix_reverse_f_issue branch from 214e73d to db2020b Compare October 11, 2025 01:41

github-actions bot added extensions protobuf and removed extensions protobuf labels Oct 11, 2025

gemini-code-assist bot reviewed Oct 11, 2025

View reviewed changes

liferoad requested a review from Abacn October 11, 2025 13:58

reuvenlax changed the title ~~tableRowFromMessage should use setF~~ Fix issues in tableRowFromMessage Oct 11, 2025

gemini-code-assist bot reviewed Oct 11, 2025

View reviewed changes

Abacn reviewed Oct 14, 2025

View reviewed changes

...-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProtoIT.java Outdated Show resolved Hide resolved

reuvenlax force-pushed the fix_reverse_f_issue branch from a7bc70c to 4141b42 Compare October 19, 2025 23:12

gemini-code-assist bot reviewed Oct 23, 2025

View reviewed changes

...ud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProto.java Outdated Show resolved Hide resolved

...ud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProto.java Show resolved Hide resolved

Abacn reviewed Oct 24, 2025

View reviewed changes

...a/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.java Outdated Show resolved Hide resolved

reuvenlax added 2 commits October 28, 2025 11:49

fix tableRowFromMessage

c6603f0

fix locale

3e65c16

reuvenlax force-pushed the fix_reverse_f_issue branch from 8f98821 to 3e65c16 Compare October 28, 2025 18:59

Abacn approved these changes Oct 30, 2025

View reviewed changes

gemini-code-assist bot reviewed Oct 30, 2025

View reviewed changes

...o/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryUtils.java Show resolved Hide resolved

reuvenlax merged commit d46a013 into apache:master Oct 31, 2025
18 of 20 checks passed

Fix issues in tableRowFromMessage #36425

Fix issues in tableRowFromMessage #36425

Uh oh!

Conversation

reuvenlax commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Oct 7, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

github-actions bot commented Oct 7, 2025

Uh oh!

Abacn left a comment

Choose a reason for hiding this comment

Uh oh!

reuvenlax commented Oct 8, 2025

Uh oh!

github-actions bot commented Oct 8, 2025

Uh oh!

liferoad commented Oct 10, 2025

Uh oh!

liferoad commented Oct 11, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

reuvenlax commented Oct 11, 2025

Uh oh!

reuvenlax commented Oct 11, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Abacn commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liferoad commented Oct 14, 2025

Uh oh!

reuvenlax commented Oct 15, 2025

Uh oh!

reuvenlax commented Oct 22, 2025

Uh oh!

reuvenlax commented Oct 23, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Abacn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Abacn left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Abacn commented Oct 30, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

reuvenlax commented Oct 7, 2025 •

edited

Loading

Abacn commented Oct 14, 2025 •

edited

Loading

Abacn left a comment •

edited

Loading